Conversation
📝 WalkthroughWalkthroughAdditive type and UI changes introducing CrowdSec support: new Changes
Sequence Diagram(s)(Skipped) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/modules/reverse-proxy/table/ReverseProxyAccessControlCell.tsx (1)
110-117: Consider applying the same IPv6 fix to existing allowed/blocked CIDR filtering.For consistency, the allowed/blocked IP filtering at lines 110-117 should also handle
/128suffixes if IPv6 support is being added across the feature.Example fix for existing filters
const allowedIps = - restrictions?.allowed_cidrs?.filter((c) => c.endsWith("/32")) ?? []; + restrictions?.allowed_cidrs?.filter((c) => c.endsWith("/32") || c.endsWith("/128")) ?? []; const allowedCidrs = - restrictions?.allowed_cidrs?.filter((c) => !c.endsWith("/32")) ?? []; + restrictions?.allowed_cidrs?.filter((c) => !c.endsWith("/32") && !c.endsWith("/128")) ?? []; const blockedIps = - restrictions?.blocked_cidrs?.filter((c) => c.endsWith("/32")) ?? []; + restrictions?.blocked_cidrs?.filter((c) => c.endsWith("/32") || c.endsWith("/128")) ?? []; const blockedCidrs = - restrictions?.blocked_cidrs?.filter((c) => !c.endsWith("/32")) ?? []; + restrictions?.blocked_cidrs?.filter((c) => !c.endsWith("/32") && !c.endsWith("/128")) ?? [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/reverse-proxy/table/ReverseProxyAccessControlCell.tsx` around lines 110 - 117, Update the CIDR filtering in ReverseProxyAccessControlCell.tsx so the allowed/blocked IP lists treat IPv6 host addresses the same as IPv4: change the .endsWith("/32") checks used to populate allowedIps and blockedIps to also accept "/128" (e.g., treat strings that end with "/32" OR "/128" as single-host IPs) and invert that logic for allowedCidrs and blockedCidrs (keep entries that do not end with "/32" AND do not end with "/128"); update the expressions that compute allowedIps, allowedCidrs, blockedIps, and blockedCidrs accordingly so IPv6 single-address CIDRs are classified consistently with IPv4.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/modules/reverse-proxy/table/ReverseProxyAccessControlCell.tsx`:
- Around line 110-117: Update the CIDR filtering in
ReverseProxyAccessControlCell.tsx so the allowed/blocked IP lists treat IPv6
host addresses the same as IPv4: change the .endsWith("/32") checks used to
populate allowedIps and blockedIps to also accept "/128" (e.g., treat strings
that end with "/32" OR "/128" as single-host IPs) and invert that logic for
allowedCidrs and blockedCidrs (keep entries that do not end with "/32" AND do
not end with "/128"); update the expressions that compute allowedIps,
allowedCidrs, blockedIps, and blockedCidrs accordingly so IPv6 single-address
CIDRs are classified consistently with IPv4.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 228346a6-219f-4254-a28b-7fc853939eb2
📒 Files selected for processing (6)
src/interfaces/ReverseProxy.tssrc/modules/reverse-proxy/ReverseProxyAccessControlRules.tsxsrc/modules/reverse-proxy/ReverseProxyModal.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsAuthMethodCell.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsReasonCell.tsxsrc/modules/reverse-proxy/table/ReverseProxyAccessControlCell.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/reverse-proxy/ReverseProxyAccessControlRules.tsx`:
- Around line 142-144: The ip rule normalization currently preserves
user-supplied CIDR suffixes which can widen access; update the logic that builds
allowed_cidrs so when rule.type === "ip" you first strip any existing CIDR
suffix from rule.value and then append the host mask (use "/128" for IPv6 when
rule.value includes ":" else "/32" for IPv4), and continue to push that
normalized value into allowed_cidrs when rule.action === "allow" (refer to the
variables rule.type, rule.value, suffix, value, and allowed_cidrs in
ReverseProxyAccessControlRules.tsx).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d8f49ff-ff0a-4c01-a546-7e18182e1263
📒 Files selected for processing (3)
src/interfaces/ReverseProxy.tssrc/modules/reverse-proxy/ReverseProxyAccessControlRules.tsxsrc/modules/reverse-proxy/table/ReverseProxyAccessControlCell.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/interfaces/ReverseProxy.ts
|
Is there a reason this hasn't been released yet with the publishing of netbird 0.69.0? The docs mention enabling it per resource, but this can't be done until the dashboard has been updated |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/modules/reverse-proxy/ReverseProxyAccessControlRules.tsx (1)
144-148:⚠️ Potential issue | 🟠 MajorIP-type rules still accept and preserve user-supplied CIDR suffixes.
validateRuleaccepts IP values with CIDR suffixes (e.g.,10.0.0.0/24) becausecidr.isValidAddresstreats CIDRs as valid, and the!rule.value.includes("/")guard here then skips the host-mask normalization, silently widening an "IP" rule into a range. Normalize unconditionally fortype === "ip": strip any existing suffix and append/32or/128.Proposed fix
- const suffix = rule.value.includes(":") ? "/128" : "/32"; - const value = - rule.type === "ip" && !rule.value.includes("/") - ? `${rule.value}${suffix}` - : rule.value; + let value = rule.value; + if (rule.type === "ip") { + const raw = value.split("/")[0]; + value = `${raw}${raw.includes(":") ? "/128" : "/32"}`; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/reverse-proxy/ReverseProxyAccessControlRules.tsx` around lines 144 - 148, The current normalization logic uses the guard !rule.value.includes("/") so user-supplied CIDR suffixes are preserved; change it so that for rule.type === "ip" you always normalize: compute suffix = rule.value.includes(":") ? "/128" : "/32", strip any existing CIDR by taking the substring before the first "/" (or otherwise remove trailing "/..."), then set value = `${strippedAddress}${suffix}`; update the assignment that currently uses the conditional ternary (referencing rule.value, suffix, and value) and ensure validateRule still accepts IP inputs before this normalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/modules/reverse-proxy/ReverseProxyAccessControlRules.tsx`:
- Around line 144-148: The current normalization logic uses the guard
!rule.value.includes("/") so user-supplied CIDR suffixes are preserved; change
it so that for rule.type === "ip" you always normalize: compute suffix =
rule.value.includes(":") ? "/128" : "/32", strip any existing CIDR by taking the
substring before the first "/" (or otherwise remove trailing "/..."), then set
value = `${strippedAddress}${suffix}`; update the assignment that currently uses
the conditional ternary (referencing rule.value, suffix, and value) and ensure
validateRule still accepts IP inputs before this normalization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55d8013d-cf23-4da3-aed7-86b765be7773
⛔ Files ignored due to path filters (1)
src/assets/integrations/crowdsec.pngis excluded by!**/*.png
📒 Files selected for processing (2)
src/modules/reverse-proxy/ReverseProxyAccessControlRules.tsxsrc/modules/reverse-proxy/ReverseProxyCrowdSecIPReputation.tsx
Add CrowdSec IP reputation integration and trusted IP support to the reverse proxy access control UI.
Issue ticket number and link
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
netbirdio/docs#698
Related PRs
Summary by CodeRabbit
New Features